Move to GH Actions by sabderemane · Pull Request #459 · python/bedevere

sabderemane · 2022-05-21T02:29:56Z

Related to #344

ezio-melotti

An early review with a few comments.

bedevere/review_pr.py

bedevere/backport.py

.github/workflows/review_pr.yml

ezio-melotti · 2022-05-21T23:57:36Z

.github/workflows/review_pr.yml

+          path: ~/.cache/pip
+          key: ${{ runner.os }}-pip-${{ hashFiles('pyproject.toml') }}
+          restore-keys: |
+            ${{ runner.os }}-pip-


Is something missing here?

I update the key but for the rest it should be ok, I use this example

actions/setup-python now natively includes pip caching.

Docs: https://github.com/actions/setup-python#caching-packages-dependencies

So we can remove the uses: actions/cache@v1 step, and update:

- - uses: actions/setup-python@v3 + - uses: actions/setup-python@v4 with: python-version: '3.x' + cache: pip + cache-dependency-path: '*requirements.txt'

And the same in the other workflows.

.github/workflows/review_pr.yml

ezio-melotti · 2022-05-21T23:58:35Z

.github/workflows/review_pr.yml

+          python-version: '3.9'
+      - name: Install Dependencies
+        run: python3 -m pip install coverage -U pip -r dev-requirements.txt
+      - name: Run code


A more descriptive name would be better.

Just to be sure, do you mean to change the name "Run code" ?

.github/workflows/review_pr.yml

arhadthedev · 2022-06-24T11:13:55Z

@sabderemane Would you mind to address the reviews, please? It would be great to retire a third party Heroku service of extra maintainance, not-so-transparent deployment, non-public error logs, and a dedicated, actively waiting server loop.

sabderemane · 2022-06-24T11:41:28Z

I will, for sure, I just didn't have time to do so yet :)

sabderemane

Thank you @ezio-melotti for the review 👍🏽

sabderemane · 2022-07-14T13:08:57Z

.github/workflows/review_pr.yml

+          path: ~/.cache/pip
+          key: ${{ runner.os }}-pip-${{ hashFiles('pyproject.toml') }}
+          restore-keys: |
+            ${{ runner.os }}-pip-


I update the key but for the rest it should be ok, I use this example

.github/workflows/review_pr.yml

bedevere/backport.py

bedevere/review_pr.py

.github/workflows/review_pr.yml

sabderemane · 2022-07-14T13:19:50Z

.github/workflows/review_pr.yml

+          python-version: '3.9'
+      - name: Install Dependencies
+        run: python3 -m pip install coverage -U pip -r dev-requirements.txt
+      - name: Run code


Just to be sure, do you mean to change the name "Run code" ?

hugovk

A few workflow suggestions :)

DanielNoord · 2022-07-14T13:35:23Z

.github/workflows/close_pr.yml

+      - uses: actions/checkout@v2
+      - uses: actions/cache@v1
+        with:
+          path: ~/.cache/pip
+          key: ${{ runner.os }}-pip-${{ hashFiles('pyproject.toml') }}
+          restore-keys: |
+            ${{ runner.os }}-pip-
+      - uses: actions/setup-python@v3
+        with:
+          python-version: '3.9'


Since we use dependabot we could also use the requirements file as the dependency path and skip the whole pip install step. That should speed up all workflows significantly.

(Oops, this posted some outdated suggestions I started before but didn't post! Deleting them, sorry for confusion!)

Dependency path: we install dev-requirements.txt, which includes installing -r requirements.txt

below I suggest using cache-dependency-path: '*requirements.txt', so if either of those files change, it invalidates the cache and downloads the new things.

hugovk · 2022-07-14T13:26:03Z

CONTRIBUTING.rst

 ==================================

-Bedevere web service is deployed to Heroku, which is managed by The PSF.
+Bedevere web service is currently launched with GitHub Actions, which is managed by The PSF.


"currently" is redundant

Suggested change

Bedevere web service is currently launched with GitHub Actions, which is managed by The PSF.

Bedevere web service is launched with GitHub Actions, which is managed by The PSF.

hugovk · 2022-07-14T13:29:19Z

.github/workflows/review_pr.yml

+          path: ~/.cache/pip
+          key: ${{ runner.os }}-pip-${{ hashFiles('pyproject.toml') }}
+          restore-keys: |
+            ${{ runner.os }}-pip-


actions/setup-python now natively includes pip caching.

Docs: https://github.com/actions/setup-python#caching-packages-dependencies

So we can remove the uses: actions/cache@v1 step, and update:

- - uses: actions/setup-python@v3 + - uses: actions/setup-python@v4 with: python-version: '3.x' + cache: pip + cache-dependency-path: '*requirements.txt'

And the same in the other workflows.

hugovk · 2022-07-14T13:29:32Z

.github/workflows/review_pr.yml

+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v2


Bump:

Suggested change

- uses: actions/checkout@v2

- uses: actions/checkout@v3

hugovk · 2022-07-14T13:30:00Z

.github/workflows/close_pr.yml

+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v2


Suggested change

- uses: actions/checkout@v2

- uses: actions/checkout@v3

hugovk · 2022-07-14T13:30:24Z

.github/workflows/backport.yml

+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v2


Bump version:

Suggested change

- uses: actions/checkout@v2

- uses: actions/checkout@v3

hugovk · 2022-07-14T13:30:32Z

.github/workflows/backport.yml

+          key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }}
+          restore-keys: |
+            ${{ runner.os }}-pip-
+      - uses: actions/setup-python@v3


Suggested change

- uses: actions/setup-python@v3

- uses: actions/setup-python@v4

hugovk · 2022-07-14T13:32:34Z

.github/workflows/filepaths.yml

+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v2


Suggested change

- uses: actions/checkout@v2

- uses: actions/checkout@v3

hugovk · 2022-07-14T13:32:40Z

.github/workflows/filepaths.yml

+          key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }}
+          restore-keys: |
+            ${{ runner.os }}-pip-
+      - uses: actions/setup-python@v3


Suggested change

- uses: actions/setup-python@v3

- uses: actions/setup-python@v4

hugovk · 2022-07-14T13:32:51Z

.github/workflows/review_pr.yml

+          key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }}
+          restore-keys: |
+            ${{ runner.os }}-pip-
+      - uses: actions/setup-python@v3


Suggested change

- uses: actions/setup-python@v3

- uses: actions/setup-python@v4

DanielNoord · 2022-07-14T13:35:23Z

.github/workflows/close_pr.yml

+      - uses: actions/checkout@v2
+      - uses: actions/cache@v1
+        with:
+          path: ~/.cache/pip
+          key: ${{ runner.os }}-pip-${{ hashFiles('pyproject.toml') }}
+          restore-keys: |
+            ${{ runner.os }}-pip-
+      - uses: actions/setup-python@v3
+        with:
+          python-version: '3.9'


Since we use dependabot we could also use the requirements file as the dependency path and skip the whole pip install step. That should speed up all workflows significantly.

ezio-melotti

I left some more review comments.

It seems that some of the changes applied lately got lost during a merge -- those should be double checked and fixed;
there were some formatting problems likely caused by a misconfigured editor (trailing spaces, tabs, changed indentation)

P.S.: you can probably batch merge most of these directly from GitHub.

ezio-melotti · 2022-07-14T17:48:32Z

.github/workflows/review_pr.yml

+          python-version: '3.9'
+      - name: Install Dependencies
+        run: python3 -m pip install coverage -U pip -r dev-requirements.txt
+      - name: Run code


ezio-melotti · 2022-07-14T17:49:26Z

CONTRIBUTING.rst

+App collaborators and members
 ''''''''''''''''''''''''''''''''''''


Suggested change

App collaborators and members

''''''''''''''''''''''''''''''''''''

App collaborators and members

'''''''''''''''''''''''''''''

ezio-melotti · 2022-07-14T17:50:29Z

bedevere/backport.py

-router = gidgethub.routing.Router()
-
-TITLE_RE = re.compile(r'\s*\[(?P<branch>\d+\.\d+)\].+\((?:GH-|#)(?P<pr>\d+)\)', re.IGNORECASE)
+TITLE_RE = re.compile(r'\s*\[(?P<branch>\d+\.\d+)\].+\((?:GH-|#)(?P<pr>\d+)\)')


Suggested change

TITLE_RE = re.compile(r'\s*\[(?P<branch>\d+\.\d+)\].+\((?:GH-|#)(?P<pr>\d+)\)')

TITLE_RE = re.compile(r'\s*\[(?P<branch>\d+\.\d+)\].+\((?:GH-|#)(?P<pr>\d+)\)', re.IGNORECASE)

ezio-melotti · 2022-07-14T17:53:06Z

bedevere/backport.py

-
-    original_issue = await gh.getitem(event.data['repository']['issues_url'],
-                                      {'number': original_pr_number})
+    


I'd recommend to configure your editor to remove trailing spaces and to add newlines at the end of the files automatically. I've also seen some tabs, so you might have to configure your editor to only use spaces too.

bedevere/close_pr.py

ezio-melotti · 2022-07-14T19:07:33Z

bedevere/backport.py

    base_branch = pull_request["base"]["ref"]

-    if not is_maintenance_branch(base_branch):
+    if base_branch == "main":


This should also use not is_maintanance_branch.
You should double check that there aren't other updates that got revert during the merge (or possibly re-do the merge).

bedevere/close_pr.py

bedevere/filepaths.py

bedevere/review_pr.py

codecov · 2022-10-03T20:47:02Z

Codecov Report

Merging #459 (77a870c) into main (909b536) will decrease coverage by 75.42%.
The diff coverage is 61.25%.

@@             Coverage Diff              @@
##              main     #459       +/-   ##
============================================
- Coverage   100.00%   24.57%   -75.43%     
============================================
  Files           18       18               
  Lines         1828     1864       +36     
  Branches       211      200       -11     
============================================
- Hits          1828      458     -1370     
- Misses           0     1401     +1401     
- Partials         0        5        +5

Flag	Coverage Δ
Python_3.10	`?`
Python_3.11-dev	`24.57% <61.25%> (-75.43%)`	⬇️
Python_3.8	`?`
Python_3.9	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
bedevere/util.py	`44.30% <0.00%> (-55.70%)`	⬇️
bedevere/backport.py	`37.80% <52.77%> (-62.20%)`	⬇️
bedevere/close_pr.py	`64.28% <71.42%> (-35.72%)`	⬇️
bedevere/filepaths.py	`60.00% <71.42%> (-40.00%)`	⬇️
tests/test_stage.py	`9.49% <0.00%> (-90.51%)`	⬇️
tests/test___main__.py	`12.00% <0.00%> (-88.00%)`	⬇️
tests/test_prtype.py	`15.04% <0.00%> (-84.96%)`	⬇️
tests/test_util.py	`17.14% <0.00%> (-82.86%)`	⬇️
tests/test_filepaths.py	`18.09% <0.00%> (-81.91%)`	⬇️
tests/test_backport.py	`18.98% <0.00%> (-81.02%)`	⬇️
... and 12 more

Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com>

sabderemane · 2022-10-03T21:50:54Z

Sorry for the late update, I have address all the issues hopefully, thank you for the review @hugovk, @ezio-melotti and @DanielNoord. I changed my configuration to avoid editor issues 🙏🏽 I will see to continue and update tests.

arhadthedev · 2023-06-15T03:51:04Z

From #344 (comment):

One issue with using Actions is that it require maintainers to "approve" the workflow for contributors before it can be run.
The workflows done by bedevere, like checking for skip issue, skip news, applying "type" labels, should just run by itself without needing maintainer approval.

So I think it would be better to convert bedevere into GitHub App instead of Actions. By converting this to GitHub App, we can resolve the issue where 2FA is needed for the bedevere-bot account.

From the following #344 (comment):

To solve the issue where core devs want to be able to re-run workflows/retrigger webhook in case the web app is down, we can grant the "GitHub App manager" access to core devs.

The problem surfaced while working on gh-568 so gh-569 was created instead.

@sabderemane Probably, your PR is a dead-end then. We need to either close it in favor of gh-569 or think on some alterations.

sabderemane · 2023-06-15T06:33:32Z

Closing in favor of gh-569, happy to help on other things instead :)

ezio-melotti reviewed May 22, 2022

View reviewed changes

sabderemane force-pushed the move-to-gh-actions branch 2 times, most recently from c90276d to 375ad52 Compare July 14, 2022 13:06

sabderemane commented Jul 14, 2022

View reviewed changes

sabderemane force-pushed the move-to-gh-actions branch from e147d18 to 447e73f Compare July 14, 2022 13:24

hugovk reviewed Jul 14, 2022

View reviewed changes

DanielNoord reviewed Jul 14, 2022

View reviewed changes

ezio-melotti requested changes Jul 14, 2022

View reviewed changes

sabderemane added 8 commits October 3, 2022 22:24

review partially backport file as a github action

3f17bce

Review backport file with maintenance part

53dec01

Update backport

5850d49

Review close_pr file and split into two files for gh actions

da58275

review filepaths

c4b1315

Review python version workflows

b0e9ef6

Review descriptions and cache workflows

ea92824

Fix indentation

195dd5f

sabderemane force-pushed the move-to-gh-actions branch from 447e73f to 195dd5f Compare October 3, 2022 20:37

sabderemane and others added 4 commits October 3, 2022 22:50

fixes from reviews

2fc8bbf

Apply suggestions from code review

11a5d34

Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com>

fixes from reviews #2

404949b

fix formatting issue

77a870c

nightlark mentioned this pull request Jan 19, 2023

bedevere-bot does not have 2FA enabled #531

Closed

sabderemane closed this Jun 15, 2023

	Bedevere web service is currently launched with GitHub Actions, which is managed by The PSF.
	Bedevere web service is launched with GitHub Actions, which is managed by The PSF.

	- uses: actions/setup-python@v3
	- uses: actions/setup-python@v4

		App collaborators and members
		''''''''''''''''''''''''''''''''''''

	TITLE_RE = re.compile(r'\s*\[(?P<branch>\d+\.\d+)\].+\((?:GH-\|#)(?P<pr>\d+)\)')
	TITLE_RE = re.compile(r'\s*\[(?P<branch>\d+\.\d+)\].+\((?:GH-\|#)(?P<pr>\d+)\)', re.IGNORECASE)


		original_issue = await gh.getitem(event.data['repository']['issues_url'],
		{'number': original_pr_number})

Uh oh!

Conversation

sabderemane commented May 21, 2022

Uh oh!

ezio-melotti left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arhadthedev commented Jun 24, 2022

Uh oh!

sabderemane commented Jun 24, 2022

Uh oh!

sabderemane left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hugovk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ezio-melotti left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

ezio-melotti left a comment •

edited

Loading

codecov bot commented Oct 3, 2022 •

edited

Loading

sabderemane commented Oct 3, 2022 •

edited

Loading